Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issue 237/email report #244

Merged
merged 17 commits into from
Oct 10, 2024
Merged

Issue 237/email report #244

merged 17 commits into from
Oct 10, 2024

Conversation

rphovley
Copy link
Member

@rphovley rphovley commented Oct 8, 2024

The Pull Request is ready

  • all github actions are passing
  • are changes backwards compatible?
  • fixes #0
  • the branch follows the naming schema issue-123-enable-x-does-not-disable-y
  • the pull request has a sensible title

Intention

With this change I intend to set some of the foundation work for the weekly report. Included some refactoring that I'll continue in another PR around establishing the new conventions for making requests to the api

Review Points

Please take extra care reviewing...

The code follows best practices

  • the code is readable
  • issues for follow-up tasks have been created
  • there is no any type used

Notes

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 8, 2024 •
edited
Loading

Visit the preview URL for this PR (updated for commit 002df4e):

https://codeclimbersio--pr244-issue-237-email-repo-ubirspfl.web.app

(expires Thu, 17 Oct 2024 01:51:43 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 3c28ab1d661a7526dc2530b2448a1aa4f88d23d4

github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 10, 2024
View reviewed changes
packages/server/src/v1/activities/activities.service.ts

await this.pulseRepo.createPulses(uniquePulses)
return activitiesUtil.pulseSuccessResponse(pulsesDto.length)
return pulseSuccessResponse(pulsesDto.length)

Check failure

Code scanning / CodeQL

Type confusion through parameter tampering Critical

Potential type confusion as
this HTTP request parameter
Loading
may be either an array or a string.

Copilot Autofix AI 5 months ago

To fix the problem, we need to ensure that the pulsesDto parameter is indeed an array of CreateWakatimePulseDto objects before processing it. This can be done by adding a type check at the beginning of the createPulses method. If the type check fails, we should throw an error or return a bad request response.

  1. Add a type check to ensure pulsesDto is an array of CreateWakatimePulseDto objects.
  2. Modify the createPulses method in activities.service.ts to include this type check.
  3. Ensure that the createPulses method returns a bad request response if the type check fails.
Suggested changeset 1
packages/server/src/v1/activities/activities.service.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/server/src/v1/activities/activities.service.ts b/packages/server/src/v1/activities/activities.service.ts
--- a/packages/server/src/v1/activities/activities.service.ts
+++ b/packages/server/src/v1/activities/activities.service.ts
@@ -45,4 +45,5 @@
   async createPulses(pulsesDto: CreateWakatimePulseDto[]) {
-    assert(!pulsesDto, 'pulsesDto required')
-    assert(!Array.isArray(pulsesDto), 'Pulses must be an array')
+    if (!Array.isArray(pulsesDto) || !pulsesDto.every(dto => dto instanceof CreateWakatimePulseDto)) {
+      throw new Error('Invalid input: pulsesDto must be an array of CreateWakatimePulseDto objects');
+    }
 
EOF
@@ -45,4 +45,5 @@
async createPulses(pulsesDto: CreateWakatimePulseDto[]) {
assert(!pulsesDto, 'pulsesDto required')
assert(!Array.isArray(pulsesDto), 'Pulses must be an array')
if (!Array.isArray(pulsesDto) || !pulsesDto.every(dto => dto instanceof CreateWakatimePulseDto)) {
throw new Error('Invalid input: pulsesDto must be an array of CreateWakatimePulseDto objects');
}

Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@rphovley rphovley merged commit 44c938b into main Oct 10, 2024
7 of 9 checks passed
@rphovley rphovley deleted the issue-237/email_report branch October 10, 2024 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@rphovley